fix(tools): write_file preserves the existing file's line-ending style (CRLF) - #5911
Conversation
|
Thanks @EvanProgramming for taking the time to contribute. This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered. Please read |
Hmbown
left a comment
Hmbown
left a comment
There was a problem hiding this comment.
Verified the bug on main — both write paths wrote content verbatim (crates/tui/src/tools/file.rs:1388, :1507) while edit_file preserved endings, so parity here is exactly right, and thanks for updating the diff/summary to use written so "(no changes)" stays truthful. Two small follow-ups: (1) normalize_contract_line_endings (file.rs:1568) also rewrites bare \r to the detected ending, so a lone CR in the new content becomes CRLF when the prior is CRLF — same policy as edit_file, but a unit test would pin it; (2) the contract path (execute_contract_write, file.rs:~1358) is only covered via the shared helper — one E2E test there would close the loop. Housekeeping: the commit is missing its Signed-off-by: trailer; mind amending with git commit --amend -s? Nice work, and thanks for the well-written #5909.
Overwriting an existing CRLF (Windows) file with write_file silently rewrote every \r\n as \n, changing byte content the model did not intend to touch and contradicting edit_file, which preserves the detected line ending. Both write paths were affected: WriteFileTool::execute and WriteFileTool::execute_contract_write. Add preserve_prior_line_endings() (normalize to LF, then restore to the prior file's detected ending) and use it in both paths. New files are written verbatim — there is no prior style to honor. Regression tests: an E2E write_file over a CRLF file stays CRLF, plus direct helper coverage for CRLF/LF/empty-prior cases. Closes Hmbown#5909. Signed-off-by: EvanProgramming <evangonggyf@gmail.com>
3e89cef to
5693fbd
Compare
|
Thanks for the review — all three follow-ups are addressed in the updated commit (
Verification: |
2 participants
Summary
Fixes #5909.
write_filesilently converted an existing CRLF (Windows) file to LF on overwrite, whileedit_filepreserves the detected line ending — so the same file behaved differently depending on the tool used. Both write paths were affected:WriteFileTool::execute(full replace)WriteFileTool::execute_contract_write(the hiddenwrite_file/Filecontract path)Root cause
Both paths wrote
file_content.as_bytes()verbatim. Model-produced content is LF, so every\r\nin the on-disk file became\n— changing byte content the model never intended to touch, corrupting diffs/blame on CRLF-committed repos and on Windows.Changes
preserve_prior_line_endings(content, prior): normalizes incoming content to LF internally, then restores the prior file's detected ending (contract_line_ending), exactly mirroringedit_file's policy.Tests
write_fileover an existing CRLF file with LF content leaves the file CRLF.Verification
cargo check -p codewhale-tui --lib --testspasses.cargo fmt -p codewhale-tuiclean (only the touched files changed).